Skip to content

[Codex] fix(logic): restrict graph mutation/run endpoints to admin users#502

Closed
Micsi wants to merge 1 commit into
abeggled:mainfrom
Micsi:codex/propose-fix-for-critical-ui-vulnerability
Closed

[Codex] fix(logic): restrict graph mutation/run endpoints to admin users#502
Micsi wants to merge 1 commit into
abeggled:mainfrom
Micsi:codex/propose-fix-for-critical-ui-vulnerability

Conversation

@Micsi

@Micsi Micsi commented May 17, 2026

Copy link
Copy Markdown
Collaborator

Motivation

  • The Logic Engine endpoints allowed any authenticated user to create, save, duplicate, import, delete, or run graphs that can contain high-impact node types (api_client, datapoint_write, python_script), enabling SSRF, device actuation, and DoS risks.
  • The intent is to restrict state-changing and execution operations to admin users while retaining read access for normal users.

Description

  • Updated obs/api/v1/logic.py to require get_admin_user instead of get_current_user for graph write/execute endpoints: POST /graphs, PUT /graphs/{id}, PATCH /graphs/{id}, DELETE /graphs/{id}, POST /graphs/import, POST /graphs/{id}/run, and POST /graphs/{id}/duplicate.
  • Kept read-only endpoints (GET /node-types, GET /graphs, GET /graphs/{id}, GET /graphs/{id}/export) using get_current_user so authenticated users retain listing and viewing capabilities.
  • Preserved existing behavior such as executor cache reload/invalidate and database interactions; no functional changes to execution or node handling were made.

Testing

  • python -m py_compile obs/api/v1/logic.py completed successfully.
  • pytest -q tests/integration/test_duplication.py could not be executed in this environment due to a missing test dependency (pytest_asyncio).

Codex Task

@Micsi Micsi added the Security Security-related changes label May 17, 2026
@abeggled

Copy link
Copy Markdown
Owner

@Micsi
Ein normaler User soll die vollen Funktionalitäten nutzen können. Der Admin hat nur die Benutzerverwaltung als zusätzliches Recht.
Ohne stichhaltigem Gegenargument werde ich den PR schliessen, da er dem gewünschten Design entspricht und kein Sicherheitsrisiko in dem Sinn darstellt.

@Micsi

Micsi commented May 17, 2026

Copy link
Copy Markdown
Collaborator Author

@abeggled , das ist komplizierter. Die Gefahren des misuse sind natürlich nicht von der Hand zu weisen, aber die grundsätliche Verwendbarkeit soll natürlich erhalten bleiben. Dennoch wieder mein Fall mit der Mietwohnung. Ich würde es begrüßen, das zunächst enger/sicherer zu machen und dann später kontrolliert im Rahmen eines konsequenten Berechtigungskonzept zu öffnen.

@abeggled abeggled added on hold dependencies on other decisions wontfix This will not be worked on and removed on hold dependencies on other decisions labels May 27, 2026
@abeggled

Copy link
Copy Markdown
Owner

Will be fixed with #583

@abeggled abeggled closed this May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Security Security-related changes wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants